Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[energidataservice] Introduce subscription-based providers #17456

Merged
merged 2 commits into from
Oct 11, 2024
Merged

[energidataservice] Introduce subscription-based providers #17456

merged 2 commits into from
Oct 11, 2024

Conversation

jlaur
Copy link
Contributor

@jlaur jlaur commented Sep 21, 2024 •
edited
Loading

Refactor the binding to use a subscription-based model for better separation of responsibilities and preparing for the future:

  • The handler is simplified and stripped of logic for polling data from the service.
  • Fetching of electricity prices and CO₂ emissions are extracted into separate providers.
  • The cache is rewritten and now has separate implementations for spot prices and tariffs.

Additionally:

  • Fix some SAT issues.

Impact of this refactoring:

  • Linking new items will no longer cause unnecessary network calls.
  • When having multiple Things, they will share the same cache which will reduce network calls and memory consumption.
  • Refresh commands will cause all subscribers to be notified if the new data is different from the already cached data.

@jlaur jlaur added enhancement An enhancement or new feature for an existing add-on additional testing preferred The change works for the pull request author. A test from someone else is preferred though. labels Sep 21, 2024
@jlaur jlaur force-pushed the energidataservice-provider branch 5 times, most recently from 25f33e7 to e12a31d Compare September 27, 2024 20:25
@jlaur jlaur marked this pull request as ready for review September 27, 2024 20:56
@jlaur jlaur removed the additional testing preferred The change works for the pull request author. A test from someone else is preferred though. label Sep 28, 2024
lsiepel
lsiepel requested changes Oct 11, 2024
View reviewed changes
Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty solid. Nice improvement. Just one comment

...va/org/openhab/binding/energidataservice/internal/provider/listener/Co2EmissionListener.java Show resolved Hide resolved
@jlaur
Introduce subscription-based providers
569e0be 
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur
Add Javadoc to public interface
d9fa094 
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
lsiepel
lsiepel approved these changes Oct 11, 2024
View reviewed changes
Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM

@lsiepel lsiepel merged commit 7666ee0 into openhab:main Oct 11, 2024
5 checks passed
@lsiepel lsiepel added this to the 4.3 milestone Oct 11, 2024
@jlaur jlaur deleted the energidataservice-provider branch October 11, 2024 14:58
joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024
@jlaur @joni1993
[energidataservice] Introduce subscription-based providers (openhab#1…
5c646ff 
…7456)

* Introduce subscription-based providers

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Oct 18, 2024
@jlaur @matchews
[energidataservice] Introduce subscription-based providers (openhab#1…
56d212b 
…7456)

* Introduce subscription-based providers

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lsiepel lsiepel lsiepel approved these changes

Assignees
No one assigned
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.
2 participants
@jlaur @lsiepel